ArC fixes for spec compatibility, round 3 #30924

Ladicek · 2023-02-06T14:09:13Z

Related to #28558

fix some bad formatting caused by the move to jakarta
add BeanContainer to the set of bean types of the BeanManager built-in bean
implement InjectionPoint.isTransient() properly
fix resolution of beans with parameterized types with type parameters with multiple bounds
validate that producers of normal-scoped beans do not have primitive or array type
ignore class-retained annotations everywhere
- I already attempted to fix issues caused by class-retained annotations once, but the CDI TCK contains tests with some pretty wild class-retained annotations, so this commit addresses that holistically
fix BeanManagerImpl.resolveObserverMethods() to return an ordered set per specification
validate that bean constructors have no @Disposes, @Observes or @ObservesAsync parameters
validate that @Dependent beans don't have conditional observer methods
unify and fix handling of transitive stereotypes
validate that producer/disposer methods don't have @Observes[Async] parameters
deprecate the io.quarkus.arc.Priority annotation
- jakarta.annotation.Priority can now be put anywhere, so io.quarkus.arc.Priority is not needed anymore

Ladicek · 2023-02-06T15:40:31Z

I know why the tests fail, fix incoming.

Ladicek · 2023-02-07T11:45:52Z

Not sure why so many checks were skipped 😕 Will rebase and try again.

mkouba · 2023-02-07T14:21:15Z

independent-projects/arc/runtime/src/main/java/io/quarkus/arc/AlternativePriority.java

- * This annotation can be used not only on bean classes, but also method and field producers (unlike pure {@code Priority}).
- *
- * @deprecated Use {@link Alternative} and {@link io.quarkus.arc.Priority}/{@link jakarta.annotation.Priority} instead
+ * @deprecated Use {@link Alternative} and {@link jakarta.annotation.Priority} instead


I wonder if we should remove this annotation instead...

I would be in favor, but ... should we first add a warning that it's deprecated for removal?

Well, it's deprecated since 2.6.0.CR1...

+1 for removal, it was meant as a temporary solution until we can make use of a standard solution anyway.

If it has been deprecated since 2.6, we can remove it. Just make sure you add an entry to the migration guide about how it should be transformed.

independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Annotations.java

manovotn

Just a few minor comments, most with my QE hat on. Otherwise looks good.

independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/BeanManagerBean.java

...ts/src/test/java/io/quarkus/arc/test/producer/illegal/NormalScopedPrimitiveProducerTest.java

...ojects/arc/tests/src/test/java/io/quarkus/arc/test/stereotypes/TransitiveStereotypeTest.java

manovotn · 2023-02-07T17:03:01Z

independent-projects/arc/runtime/src/main/java/io/quarkus/arc/AlternativePriority.java

- * This annotation can be used not only on bean classes, but also method and field producers (unlike pure {@code Priority}).
- *
- * @deprecated Use {@link Alternative} and {@link io.quarkus.arc.Priority}/{@link jakarta.annotation.Priority} instead
+ * @deprecated Use {@link Alternative} and {@link jakarta.annotation.Priority} instead


+1 for removal, it was meant as a temporary solution until we can make use of a standard solution anyway.

manovotn · 2023-02-07T17:05:01Z

independent-projects/arc/runtime/src/main/java/io/quarkus/arc/Priority.java

 * <p>
 * A priority specified by {@link AlternativePriority} and {@link jakarta.annotation.Priority} takes precedence.
+ *
+ * @deprecated use {@link jakarta.annotation.Priority} instead


We could log a warning/info for this annotation to help users spot it before we get to removal, WDYT?

I don't think anyone looks at the warnings we currently produce, and adding more is just an exercise in futility.

What we could do is fail the build if the annotation is used, but that's a pretty harsh measure :-)

We have done that before and we can just drop the annotation if we document it properly in the migration guide.

It has been deprecated for a long time and in several Quarkus and RHBQ releases.

Fair enough. I don't think this PR is a good vehicle for that, as it affects a few Quarkus core extensions and at least one Quarkiverse extension. I filed #30963 for it.

gsmet · 2023-02-08T09:21:11Z

CI failures don't look related.

I filled #30988 for the first one and created a PR to disable the MongoDB test on Windows again.

…ilt-in bean

…eters with multiple bounds

…tive or array type In theory, this validation should only fail if there's an injection point that resolves to the bean. However, we do all similar validations just on the producer declaration; this commit only follows suit.

…d set per specification

…@ObservesAsync parameters

…ethods

This commit fixes handling of transitive stereotypes in these 4 areas: - inheriting bean scope from stereotypes - inheriting default bean name status from stereotypes - inheriting alternative status from stereotypes - inheriting priority from stereotypes Previously, some of these areas only looked one level deep into transitive stereotypes, while others did not consider transitive stereotypes at all. They are all consistent now.

quarkus-bot · 2023-02-08T17:16:14Z

✔️ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

Ladicek requested review from mkouba and manovotn February 6, 2023 14:09

Ladicek mentioned this pull request Feb 6, 2023

CDI Lite compliance (CDI 4.0) #28558

Closed

quarkus-bot bot added the area/arc Issue related to ARC (dependency injection) label Feb 6, 2023

Ladicek force-pushed the arc-fixes branch from afe809c to 595ff1a Compare February 6, 2023 15:49

This comment has been minimized.

Sign in to view

Ladicek force-pushed the arc-fixes branch from 595ff1a to 7f65130 Compare February 7, 2023 08:46

This comment has been minimized.

Sign in to view

Ladicek force-pushed the arc-fixes branch from 7f65130 to 03a76b3 Compare February 7, 2023 11:46

mkouba reviewed Feb 7, 2023

View reviewed changes

independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Annotations.java Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

Ladicek force-pushed the arc-fixes branch from 03a76b3 to 2643a7e Compare February 7, 2023 14:45

manovotn reviewed Feb 7, 2023

View reviewed changes

This comment has been minimized.

Sign in to view

Ladicek added 12 commits February 8, 2023 12:40

ArC: improve recent reformatting

d7164d3

ArC: add BeanContainer to the set of bean types of the BeanManager bu…

e055c10

…ilt-in bean

ArC: implement InjectionPoint.isTransient() properly

64de959

ArC: fix resolution of beans with parameterized types with type param…

9b49f56

…eters with multiple bounds

ArC: ignore class-retained annotations everywhere

846de2a

ArC: fix BeanManagerImpl.resolveObserverMethods() to return an ordere…

9046df8

…d set per specification

ArC: validate that bean constructors have no @Disposes, @observes or …

9306db1

…@ObservesAsync parameters

ArC: validate that @dependent beans don't have conditional observer m…

831c013

…ethods

ArC: improve validation of producer, disposer and observer methods

e9baeb1

ArC: deprecate the io.quarkus.arc.Priority annotation

c8200da

Ladicek force-pushed the arc-fixes branch from 2643a7e to c8200da Compare February 8, 2023 12:15

manovotn approved these changes Feb 8, 2023

View reviewed changes

mkouba approved these changes Feb 8, 2023

View reviewed changes

manovotn added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Feb 8, 2023

gsmet merged commit a7ebe03 into quarkusio:main Feb 8, 2023

quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Feb 8, 2023

quarkus-bot bot added this to the 3.0 - main milestone Feb 8, 2023

Ladicek deleted the arc-fixes branch February 8, 2023 18:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ArC fixes for spec compatibility, round 3 #30924

ArC fixes for spec compatibility, round 3 #30924

Ladicek commented Feb 6, 2023

Ladicek commented Feb 6, 2023

This comment has been minimized.

This comment has been minimized.

Ladicek commented Feb 7, 2023

mkouba Feb 7, 2023

Ladicek Feb 7, 2023

mkouba Feb 7, 2023

manovotn Feb 7, 2023

gsmet Feb 7, 2023 •

edited

Loading

This comment has been minimized.

manovotn left a comment

manovotn Feb 7, 2023

manovotn Feb 7, 2023

Ladicek Feb 7, 2023

gsmet Feb 7, 2023

Ladicek Feb 7, 2023

This comment has been minimized.

gsmet commented Feb 8, 2023

quarkus-bot bot commented Feb 8, 2023

ArC fixes for spec compatibility, round 3 #30924

ArC fixes for spec compatibility, round 3 #30924

Conversation

Ladicek commented Feb 6, 2023

Ladicek commented Feb 6, 2023

This comment has been minimized.

This comment has been minimized.

Ladicek commented Feb 7, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gsmet Feb 7, 2023 • edited Loading

Choose a reason for hiding this comment

This comment has been minimized.

manovotn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

gsmet commented Feb 8, 2023

quarkus-bot bot commented Feb 8, 2023

gsmet Feb 7, 2023 •

edited

Loading